Prevent crash on ARM32 #1412
Prevent crash on ARM32 #1412
Conversation
This change has 2 parts, one which is JIT part and likely require submission to runtime. I keep that change here to give full scope of changes. Will extract if everything would be fine. JIT for ARM now start numering floating point registers starting from 256 Second part is DWARF generation In order to produce DWARF I made following changes - Account for register numbers more then 31, by use DW_OP_bregx and DW_OP_regx for large regnum - Combine VLT_FPSTK and VLT_STK handling. See dotnet#1388
| @@ -331,6 +367,7 @@ static void EmitVarLocation(MCObjectStreamer *Streamer, | |||
| IsByRef = true; | |||
| case ICorDebugInfo::VLT_STK2: | |||
| IsStk2 = true; | |||
| case ICorDebugInfo::VLT_FPSTK: | |||
There was a problem hiding this comment.
Did you actually hit this case? I would not expect it to be hit on. Floating point stack is a legacy 32-bit x86 that are not using anymore even on 32-bit x86.
There was a problem hiding this comment.
It is hit. I was tired of seeing assertion. This change was fishy to me too. But I do not know where to start looking. With that hint, I try to pin-point what can cause code to hit that location.
There was a problem hiding this comment.
I have looked around. VLT_FPSTK is actually used for regular local variables that do not live on floating point stack. It does not look right; but it may done that way for some legacy reasons. So your change is ok.
| Streamer->EmitIntValue(DwarfRegNum + dwarf::DW_OP_breg0, 1); | ||
| } | ||
| else { | ||
| Streamer->EmitIntValue(dwarf::DW_OP_bregx, 1); |
There was a problem hiding this comment.
According to the dwarf manual: "The DW_OP_bregx operation has two operands: a register which is specified by an unsigned LEB128 number, followed by a signed LEB128 offset.". It would be nice to emit the offset as part of this function instead of emitting it in the calling code.
Co-authored-by: Jan Kotas <[email protected]>
src/coreclr/jit/unwind.cpp
Outdated
| #else | ||
| regNum = REG_NEXT(regNum), | ||
| #endif | ||
| regBit <<= 1) |
There was a problem hiding this comment.
Does this need to shift the bit by 2 for floats as well?
|
@jkotas If you don't have any other suggestions, can I split this PR on two parts? One for runtime, second for this repo. |
|
I do not have any other comments.
Yes, it would be nice. Thank you! |
|
Superseeded by #1413 and dotnet/runtime#57443 |
This change has 2 parts, one which is JIT part and likely require submission to runtime.
I keep that change here to give full scope of changes. Will extract if everything would be fine.
JIT for ARM now start numering floating point registers starting from 256
Second part is DWARF generation
In order to produce DWARF I made following changes
See #1388